Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SEGFAULT in BMI when removing a port #724

Merged
merged 1 commit into from
Mar 2, 2019

Conversation

antoninbas
Copy link
Member

There were 2 issues with the code:

  • The FD_CLR call had been moved after the memset of the bmi_port_t
    structure, which means that we were not clearing the file descriptor
    at all.
  • We should not close the file descriptor while the select call is
    on-going, it is undefined behavior. We introduce a socketpair so that
    the thread removing the port can notify the select thread and wait
    for an acknowledgement before closing the file descriptor.

There were 2 issues with the code:
 * The FD_CLR call had been moved after the memset of the bmi_port_t
   structure, which means that we were not clearing the file descriptor
   at all.
 * We should not close the file descriptor while the select call is
   on-going, it is undefined behavior. We introduce a socketpair so that
   the thread removing the port can notify the select thread and wait
   for an acknowledgement before closing the file descriptor.
@codecov-io
Copy link

codecov-io commented Mar 2, 2019

Codecov Report

Merging #724 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #724      +/-   ##
==========================================
+ Coverage   74.68%   74.69%   +0.01%     
==========================================
  Files         115      115              
  Lines        9894     9894              
==========================================
+ Hits         7389     7390       +1     
+ Misses       2505     2504       -1
Impacted Files Coverage Δ
src/bm_sim/learning.cpp 82.17% <0%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 942a9ba...7ab11ac. Read the comment docs.

@antoninbas antoninbas merged commit 5d25d0d into master Mar 2, 2019
@antoninbas antoninbas deleted the antonin/fix-segfault-in-BMI-when-removing-port branch March 2, 2019 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants